update legacy react lifecycle methods to indicate unsafe status #610
Conversation
Following the react migration path laid out here https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com>
|
Excuse my lack of React knowledge, but what would this accomplish? Wouldn't this result in the methods not being called when needed? |
Codecov Report
@@ Coverage Diff @@
## master #610 +/- ##
==========================================
+ Coverage 92.69% 92.70% +0.01%
==========================================
Files 227 227
Lines 5911 5911
Branches 1491 1491
==========================================
+ Hits 5479 5480 +1
+ Misses 391 390 -1
Partials 41 41
Continue to review full report at Codecov.
|
Actually, somewhat the opposite. In the future, the UNSAFE_* versions of these methods are the only ones that will be called. According to to the "Gradual Migration Plan" at https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html, in react 16.3, these method names are simply aliases (although, IMO the UNSAFE_* is a nice visual indicator that there is some debt hanging around if nothing else). In later versions of 16, the legacy methods (like componentDidMount), will throw a deprecated warning in development mode, whereas the UNSAFE_* ones will not. UNSAFE_* in this case acknowledging, "yes we're doing it the old way, and it's with intent" In React 17+, the legacy methods (like componentDidMount) will not be called at all. Only the UNSAFE_* version will be executed in the component lifecycle. If you're looking for a short list of benefits as I see them, this change allows the project to upgrade React through 16.X and into 17.X without any major code changes, while still giving a pretty clear indicator that there is potential refactoring to be done. This would allow component refactoring to happen at a schedule decoupled from any needed React upgrades. |
|
👍 |
|
@rubenvp8510 would you be able to review? |
|
This LGTM as a starting point to address the migration of those methods. |
|
Thanks! |
…ertracing#610) Following the react migration path laid out here https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
…ertracing#610) Following the react migration path laid out here https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
…ertracing#610) Following the react migration path laid out here https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Following the react migration path laid out here https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html
Which problem is this PR solving?
Short description of the changes